Skip to content

Conversation

@samanklesaria
Copy link
Collaborator

This PR wraps the load_with_torchcodec and save_with_torchcodec functions with functions of the name load and save so that code that depends on the old load and save functions can continue to work in the future once we remove backend-specific code.

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 12, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/audio/4039

Note: Links to docs will display an error until the docs builds have been completed.

❌ 8 New Failures

As of commit 498ce49 with merge base 02351a6 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed label Aug 12, 2025
@samanklesaria
Copy link
Collaborator Author

Installing ffmpeg>4, which is necessary for torchcodec to be able to load files used during testing, seems to be incompatible with the current CI infrastructure. Perhaps we need a separate PR to install ffmpeg>4, and wait for the infrastructure to improve so that that PR can be merged.

@samanklesaria
Copy link
Collaborator Author

samanklesaria commented Aug 14, 2025

I'm coming across an interesting discrepancy in the load behavior here. In test_load_save_torchcodec.py, the function test_save_channels_first creates a random tensor called waveform. When I try it, I get waveform[0,1345] = -4.0688. Saving and loading the result with scipy gives waveform_ta[0,1345] = -4.0688. But saving with torchcodec and loading with scipy gives waveform[0,1345] = -1.. What exactly is happening here? Are we supposed to be truncating saved values between -1 and 1? Is the normalization that torchcodec is doing different than the normalization scipy does?
I guess the docstring for save_with_torchcodec says "TorchCodec AudioEncoder expects float32 samples in [-1, 1] range". Does this mean the behavior of this test is undefined when our random waveform has larger values?

An easy fix here is just to use rand instead of randn to initialize our random tensor.

@samanklesaria samanklesaria marked this pull request as ready for review August 14, 2025 03:46
@samanklesaria samanklesaria requested a review from a team as a code owner August 14, 2025 03:46
@NicolasHug
Copy link
Member

Your assessment on test_load_save_torchcodec.py are correct, it's would have been best to ensure that input values of the encoder are in [-1, 1] rather than to use randn.

However, this entire file is meant to test the old torchaudio load() and save() against load_with_torchcodec() and save_with_torchcodec()

Now that we are moving load() and save() to rely on load_with_torchcodec() and save_with_torchcodec() directly, these tests aren't worth running anymore. Even more, we are switching torchcodec for scipy - so these tests are not testing what they were meant to test anymore.

Basically, we can just skip them safely. Just add a big pytest.skip() statement at the top, indicating this test file was only relevant back when load_with_torchcodec() and save_with_torchcodec() were introduced and load() and save() still had their own implementation.

@samanklesaria samanklesaria marked this pull request as draft August 14, 2025 14:58
@samanklesaria
Copy link
Collaborator Author

samanklesaria commented Aug 14, 2025

Now we're failing because core dumped) python -c "import torch; import torchaudio; import torchcodec; print(torch.__version__, torchaudio.__version__, torchcodec.__version__)" fails. Because I never defined __version__ in my mock module. But do I need it? It might be easiest to remove this check in the install script.

@samanklesaria
Copy link
Collaborator Author

samanklesaria commented Aug 14, 2025

I'm going to remove the installation of torchcodec during testing, as it shouldn't be used anyway. We should rely on the mock.

@samanklesaria samanklesaria marked this pull request as ready for review August 15, 2025 16:46
@NicolasHug NicolasHug changed the title Torchcodec loading Let torchaudio.load() and torchaudio.save() rely on load_with_torchcodec() and save_with_torchcodec(). Aug 18, 2025
from typing import Union, BinaryIO, Optional, Tuple
import os
import torch
import sys
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys doesn't seem to be used

@NicolasHug NicolasHug merged commit 93f582c into main Aug 18, 2025
42 of 43 checks passed
@NicolasHug NicolasHug deleted the torchcodec_loading branch August 18, 2025 12:29
@mergennachin
Copy link

mergennachin commented Sep 10, 2025

@NicolasHug @samanklesaria

we just updated our torchaudio pin to 09/06 (as we're preparing to update the pin to release branch) and it is now saying torchcodec is not found.

We can add torchcodec now in our CI and dependencies but shouldn't the proper fix be to install torchcodec as part of install_requires=[] in setup.py?

See error below

```

2025-09-10T11:41:20.8448305Z adding 'executorch-0.8.0a0+4d6209b.dist-info/METADATA'
2025-09-10T11:41:20.8448724Z adding 'executorch-0.8.0a0+4d6209b.dist-info/WHEEL'
2025-09-10T11:41:20.8449214Z adding 'executorch-0.8.0a0+4d6209b.dist-info/entry_points.txt'
2025-09-10T11:41:20.8449762Z adding 'executorch-0.8.0a0+4d6209b.dist-info/top_level.txt'
2025-09-10T11:41:20.8450200Z adding 'executorch-0.8.0a0+4d6209b.dist-info/RECORD'
2025-09-10T11:41:20.8450590Z removing pip-out/bdist.linux-x86_64/wheel
2025-09-10T11:41:20.8451280Z Building wheel for executorch (pyproject.toml) ... �[?25l�[?25hdone
2025-09-10T11:41:20.8452290Z Created wheel for executorch: filename=executorch-0.8.0a0+4d6209b-cp310-cp310-linux_x86_64.whl size=10726374 sha256=eaaec542e8d203515a8a054a589e7a1d854744e59611c47bd60e37ce0b6f969f
2025-09-10T11:41:20.8453591Z Stored in directory: /tmp/pip-ephem-wheel-cache-xgphcbw2/wheels/9e/f0/2b/6a778c77421b91e006bef425e288a1e5c7c35b04c51317756b
2025-09-10T11:41:20.8454293Z Successfully built executorch
2025-09-10T11:41:20.8454611Z Installing collected packages: executorch
2025-09-10T11:41:20.8454956Z Attempting uninstall: executorch
2025-09-10T11:41:20.8455315Z Found existing installation: executorch 0.8.0a0+4d6209b
2025-09-10T11:41:20.8455766Z Uninstalling executorch-0.8.0a0+4d6209b:
2025-09-10T11:41:20.8456166Z Removing file or directory /opt/conda/envs/py_3.10/bin/flatc
2025-09-10T11:41:20.8456923Z Removing file or directory /opt/conda/envs/py_3.10/lib/python3.10/site-packages/executorch-0.8.0a0+4d6209b.dist-info/
2025-09-10T11:41:20.8457856Z Removing file or directory /opt/conda/envs/py_3.10/lib/python3.10/site-packages/executorch/
2025-09-10T11:41:20.8458447Z Successfully uninstalled executorch-0.8.0a0+4d6209b
2025-09-10T11:41:20.8458890Z changing mode of /opt/conda/envs/py_3.10/bin/flatc to 755
2025-09-10T11:41:20.8459307Z Successfully installed executorch-0.8.0a0+4d6209b
2025-09-10T11:41:20.8459757Z + python -m unittest examples.models.moshi.mimi.test_mimi
2025-09-10T11:41:20.8460213Z
2025-09-10T11:41:20.8460692Z tokenizer-e351c8d8-checkpoint125.safeten(…): 0% 0.00/385M [00:00<?, ?B/s]
2025-09-10T11:41:20.8461461Z tokenizer-e351c8d8-checkpoint125.safeten(…): 0% 46.9k/385M [00:00<1:10:09, 91.4kB/s]
2025-09-10T11:41:20.8462240Z tokenizer-e351c8d8-checkpoint125.safeten(…): 4% 15.2M/385M [00:00<00:11, 33.0MB/s]
2025-09-10T11:41:20.8463013Z tokenizer-e351c8d8-checkpoint125.safeten(…): 19% 72.0M/385M [00:00<00:01, 157MB/s]
2025-09-10T11:41:20.8463765Z tokenizer-e351c8d8-checkpoint125.safeten(…): 34% 131M/385M [00:00<00:00, 261MB/s]
2025-09-10T11:41:20.8464521Z tokenizer-e351c8d8-checkpoint125.safeten(…): 67% 256M/385M [00:00<00:00, 510MB/s]
2025-09-10T11:41:20.8465262Z tokenizer-e351c8d8-checkpoint125.safeten(…): 100% 385M/385M [00:01<00:00, 377MB/s]
2025-09-10T11:41:20.8465744Z E
2025-09-10T11:41:20.8465960Z ======================================================================
2025-09-10T11:41:20.8466414Z ERROR: setUpClass (examples.models.moshi.mimi.test_mimi.TestMimiModel)
2025-09-10T11:41:20.8466911Z ----------------------------------------------------------------------
2025-09-10T11:41:20.8467278Z Traceback (most recent call last):
2025-09-10T11:41:20.8467923Z File "/var/lib/ci-user/.local/lib/python3.10/site-packages/torchaudio/_torchcodec.py", line 81, in load_with_torchcodec
2025-09-10T11:41:20.8468687Z from torchcodec.decoders import AudioDecoder
2025-09-10T11:41:20.8469090Z ModuleNotFoundError: No module named 'torchcodec'
2025-09-10T11:41:20.8469350Z
2025-09-10T11:41:20.8469574Z The above exception was the direct cause of the following exception:
2025-09-10T11:41:20.8469901Z
2025-09-10T11:41:20.8470011Z Traceback (most recent call last):
2025-09-10T11:41:20.8470549Z File "/pytorch/executorch/examples/models/moshi/mimi/test_mimi.py", line 88, in setUpClass
2025-09-10T11:41:20.8471317Z cls.sample_pcm, cls.sample_sr = read_mp3_from_url(
2025-09-10T11:41:20.8471987Z File "/pytorch/executorch/examples/models/moshi/mimi/test_mimi.py", line 52, in read_mp3_from_url
2025-09-10T11:41:20.8472683Z waveform, sample_rate = torchaudio.load(audio_stream, format="mp3")
2025-09-10T11:41:20.8473335Z File "/var/lib/ci-user/.local/lib/python3.10/site-packages/torchaudio/init.py", line 95, in load
2025-09-10T11:41:20.8473902Z return load_with_torchcodec(
2025-09-10T11:41:20.8474518Z File "/var/lib/ci-user/.local/lib/python3.10/site-packages/torchaudio/_torchcodec.py", line 83, in load_with_torchcodec
2025-09-10T11:41:20.8475204Z raise ImportError(
2025-09-10T11:41:20.8475733Z ImportError: TorchCodec is required for load_with_torchcodec. Please install torchcodec to use this function.
2025-09-10T11:41:20.8476245Z
2025-09-10T11:41:20.8476394Z ----------------------------------------------------------------------

</details>

@NicolasHug
Copy link
Member

Hi @mergennachin ,

torchaudio.load() and torchaudio.save() are "morally deprecated" in the sense that we encourage users to rely on native TorchCodec functionality instead of using them directly. We're only keeping them (with torchcodec as the backend) for convenience and because removing them would probably be too disruptive.

But ultimately, we still want users (e.g. possibly Executorch) to move away from those, and take a hard, explicit dependency on TorchCodec. What you're doing in pytorch/executorch#14147 is a good temporary workaround.

As a result we're not adding TorchCodec as a dependency of TorchAudio: only the users of load() and save() would need it, and we're trying to discourage their direct usage. I hope this makes sense!

swolchok pushed a commit to pytorch/executorch that referenced this pull request Sep 10, 2025
StrycekSimon pushed a commit to nxp-upstream/executorch that referenced this pull request Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants